Repository navigation
Conversation
The data given to g_idle_add(), g_timeout_add() and their _seconds/_full variants is passed to the callback (and the destroy notify), which usually frees it. With <leak-ignore/> every such call was reported as Memory leak: data Configure them as <use> instead.
g_main_context_invoke() and g_main_context_invoke_full() pass their data to the function (and the destroy notify), which usually frees it, like g_idle_add(). With <leak-ignore/> every such call was reported as Memory leak: data Configure them as <use> and describe their arguments.
dmcppcheck
left a comment
dmcppcheck
left a comment
There was a problem hiding this comment.
AI review
This PR causes a new false negative: leaks of a GMainContext passed to g_main_context_invoke / g_main_context_invoke_full are no longer reported. CheckLeakAutoVarImpl::functionCall applies a <use> function to every plain pointer variable in the call, because <use> has no argument number. So g_main_context_invoke(ctx, cb, NULL) now marks the context in ctx as taken over. That context comes from g_main_context_new(), which is in the g_main_context_unref group. GLib does not take ownership of the context; it only refs it internally. The base config reports Memory leak: ctx for this code; the PR does not. g_idle_add* and g_timeout_add* don't have this problem, because their only pointer parameters are the callback, the data and the destroy notify.
The memleak false positives that the PR is about are gone, and test/cfg/gtk.c passes. The new <arg> configuration for g_main_context_invoke* works: it reports a NULL callback as nullPointer and an uninitialized callback or priority as uninitvar.
Testing
Build: make -j8 cppcheck testrunner in pr/ succeeded.
cfg test: I ran test/cfg/gtk.c with the runtests.sh options (--check-library --platform=unix64 --enable=style,information --inconclusive --force --check-level=exhaustive --inline-suppr ... --library=gtk).
- With the PR's
gtk.cfg: clean, exit code 0. - With the base
gtk.cfg: 8memleakerrors in the new test functions. So the test does cover the change.
Regression found (I compared the PR's cfg with a copy of base's gtk.cfg, both run with the PR binary, since only the cfg changed):
static gboolean cb(gpointer data) { g_free(data); return FALSE; }
void ctx_leak(void)
{
GMainContext *ctx = g_main_context_new();
g_main_context_invoke(ctx, cb, NULL);
}
void ctx_ok(void)
{
GMainContext *ctx = g_main_context_new();
g_main_context_invoke(ctx, cb, NULL);
g_main_context_unref(ctx);
}$ cppcheck -q --library=/tmp/b/gtkbase.cfg t2.c
t2.c:16:1: error: Memory leak: ctx [memleak]
$ cppcheck -q --library=pr/cfg/gtk.cfg t2.c
(no output)
g_main_context_invoke_full(ctx, 0, cb, NULL, NULL) behaves the same way: base reports the leak, the PR does not.
Other checks:
g_main_context_invoke(NULL, NULL, NULL)now givesnullPointer, and an uninitialized callback or priority givesuninitvar.- A
malloc'd org_new'd pointer passed as the data is no longer reported, and no mismatch is reported either. - A conditional
if (x) g_idle_add(cb, p);is no longer reported. That matches howif (x) g_free(p);is already handled, so I don't count it as a problem of this PR.
Automated review of 51b97a0 by Claude. The pull request was built and tested locally.
| <use>g_register_data</use> | ||
| <use>g_idle_add</use> | ||
| <use>g_idle_add_full</use> | ||
| <use>g_main_context_invoke</use> |
There was a problem hiding this comment.
AI review
<use> has no argument number. CheckLeakAutoVarImpl::functionCall treats every plain pointer variable in a non-leak-ignore call as taken over, and that includes the GMainContext *context argument. As a result, this leak is no longer found (base reports Memory leak: ctx, the PR reports nothing):
void f(void) {
GMainContext *ctx = g_main_context_new();
g_main_context_invoke(ctx, cb, NULL);
}The same happens with g_main_context_invoke_full. GLib does not take ownership of the context. Consider adding a test/cfg/gtk.c case with // cppcheck-suppress memleak for this, and fixing it in the library or check, for example with a per-argument <use>/ownership setting. Otherwise, consider leaving the g_main_context_invoke* functions out of this change.
I don't have a Trac account, so there is no ticket for this; the details are below. Feel free to create one and I'll add it to the title.
GLib passes the data to the callback, and to the destroy notify of the
_fullvariants, which commonly frees it, so there is no leak. gtk.cfg marked these functions<leak-ignore/>:g_idle_add,g_idle_add_fullg_timeout_add,g_timeout_add_seconds,g_timeout_add_full,g_timeout_add_seconds_fullg_main_context_invoke,g_main_context_invoke_fullThey are now
<use>in theg_freememory group, without<leak-ignore/>.g_main_context_invokeandg_main_context_invoke_fullalso get their argument configuration, which they did not have.This is the gtk.cfg change that #8953 (#15137) was made for: without that fix,
--check-librarystill reportedcheckLibraryUseIgnorefor these functions. Here<use>means what the reference describes: the function takes over the pointer.Tests:
test/cfg/gtk.ccovers all eight functions; the memleak false positives show without the change.cfg/gtk.cfgvalidates againstcppcheck-cfg.rng.